To me, this problem is solved with minimal code changes to worker manager to allow for a secondary worker config to be used when provisioning workers to fill the minCapacity void. This allows us to just use what's already there.

This could look like allowing a second block of worker config in the worker pool config or even just have worker manager automatically edit the idleTimeoutSecs to 0 and make it an on-demand instance type for the workers they're provisioning to get the capacity to minCapacity.

I think this proposal is in really good shape from my point of view 👍 thank you

I understand the motivation for splitting a worker pool into two groups - the workers that stay alive forever, and those that don't, but I see some downsides to this approach. I think the reason this solution is proposed is because currently workers decide themselves when to terminate, rather than being signaled to do by a central authority (e.g. Worker Manager). However, at this point, Worker Manager is now effectively managing termination of some instances and not others, and under this approach, the responsibility is somewhat artificially split, with neither party (Worker Manager and the the worker itself) having control of the entire pool. This leads to some unpleasant consequences, such as worker manager needing to hack the worker config generation (the worker config is currently embedded in the launch config, and will not map to what the worker config actually looks like on the worker, since the idle timeout parameter only applies to a subset of the workers). Note, different worker implementations may also have different config settings for the idle timeout, so Worker Manager needing to understand the config of different worker implementations is also very brittle. It also doesn't allow Worker Manager to be smart about which workers it wants to keep alive, and those that it wants to terminate, since it can only terminate the workers in the "stay alive forever" group, which might be suboptimal (perhaps it is preferential to retain a different subset of workers that are cheaper/more performant). For example, prices might drop for one instance type, but you have workers set to run forever with a different instance type, so you are stuck with the ones you have.

To solve all of this, since Worker Manager is now becoming responsible for managing the termination of some workers, I think it would be better to move the entire responsibility to Worker Manager, and avoid the hacking of the Generic Worker config. I would propose that rather than quarantining the worker (which is intended for a different use case) and then later shutting it down, that a signal of some sort would be sent that signals that the worker should gracefully terminate. This way we do not muddy the Quarantine logs with events that are for terminations, not for quarantines (e.g. auditing real quarantines becomes problematic when the audit logs are full of business-as-usual terminations).

Consider also that technically a worker can perform work for a worker pool even if it wasn't launched by Worker Manager. Worker Manager may know it exists (since the Queue will report the worker as being active) and it may contribute towards the pool size, but maybe Worker Manager does not have a means to terminate it. My point here is that it gets very messy very quickly trying to account for all scenarios in this proposed design. I think it would be much simpler and cleaner instead for Worker Manager to make decisions about all the workers it launches, and move the responsibility to Worker Manager in a worker-implementation-agnostic way (Worker Manager should not need to know what the worker implementation config settings are or how to patch them) and at a worker pool level we should be able to tell Worker Manager "keep this many workers alive" and it should be able to signal to workers it has created that it no longer needs them, whenever it wants, and use this simple mechanism to manage its pool, not just a subset of the workers it has created.

So in summary, I think we should:

1. When generating worker pool definitions for Generic Worker workers in community-tc-config, fxci-config etc, set idle time to 0 (the config option can continue to be supported by Generic Worker, but just not used in our Worker Manager managed worker pools, as it is potentially still useful for when the worker isn't spawned by Worker Manager).

2. introduce a signalling mechanism that Worker Manager can use to advise the worker to stop taking new jobs, and to shutdown (similar to quarantine but more explicit that it should be followed by a shutdown). Avoid hijacking the existing call which is really intended for auditing (rare) security incidents, troubleshooting unexpected behaviour etc.

3. Worker Manager implements some criteria to decide which workers to terminate and which to keep alive when pool size is greater than desired. Initially this could be just keeping the workers around that have been alive the longest, but there could be some scoring mechanism which favours long lived workers but also measures performance, tracks anomolies, or unexpected expired task claims, etc. Maybe there is a cap on maximum lifetime of a worker for security reasons (e.g. don't allow any worker to live longer than 2 weeks) which may also ensure images are not too old (e.g. if the machine image it is built from becomes out of date, it should not be kept indefinitely). There are several reasons why your initial choice about which workers to keep alive indefinitely might change, and having a homogeneous pool where "all workers are equal" in the ability to keep workers alive, gives Worker Manager more power to make smarter decisions, avoid muddying configs, and keeping split of responsibilities clear.